Support direct route connection in AppGraph #7072
Conversation
Signed-off-by: Young Bu Park <[email protected]>
Signed-off-by: Young Bu Park <[email protected]>
youngbupark
temporarily deployed
to
functional-tests
GitHub Actions
Inactive
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
youngbupark
changed the title
| }) | ||
|
|
||
| return entries | ||
| } | ||
|
|
||
| // findSourceResource finds resource id by using source string by the following criteria: |
There was a problem hiding this comment.
This PR is to support direct route scenario. The wrong connection direction bug will be fixed once this is merged.
Signed-off-by: Young Bu Park <[email protected]>
youngbupark
changed the title
youngbupark
temporarily deployed
to
functional-tests
GitHub Actions
Inactive
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
| { | ||
| "connections": [ | ||
| { | ||
| "direction": "Inbound", |
There was a problem hiding this comment.
So this is definitely the wrong direction. Is that tracked by a separate issue?
There was a problem hiding this comment.
So this is definitely the wrong direction. Is that tracked by a separate issue?
#7038 is the tracking issue for this bug.
| "type": "Applications.Core/containers" | ||
| }, | ||
| { | ||
| "connections": [], |
There was a problem hiding this comment.
@nithyatsu should the connection also appear here?
There was a problem hiding this comment.
IICR the data structure annotates the connection in both directions so it's easy to display.
Here's the CLI code that displays it: https://github.com/radius-project/radius/blob/main/pkg/cli/cmd/app/connections/display.go
There was a problem hiding this comment.
This is related to connection bug. When I fixed Inbound direction bug, the connection was generated here. So this will be resolved in the following PR.
There was a problem hiding this comment.
I'm ok with that as long as it's going to be fixed soon.
| // MustUnmarshalFromFile reads testdata and unmarshals it to the given type, panicking if an error occurs. | ||
| func MustUnmarshalFromFile(file string, out any) { | ||
| dec := json.NewDecoder(bytes.NewReader(ReadFixture(file))) | ||
| dec.DisallowUnknownFields() |
Signed-off-by: Young Bu Park <[email protected]>
youngbupark
temporarily deployed
to
functional-tests
GitHub Actions
Inactive
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
| @@ -497,21 +497,57 @@ func connectionsFromAPIData(resource generated.GenericResource) []*corerpv202310 | |||
| data := corerpv20231001preview.ConnectionProperties{} | |||
There was a problem hiding this comment.
This seems to be the place where we are marking the dierction wrong, connection is an outbound correct?
containerA{ .. connection: { source} ...} => containerA ---> containerB as a service
# Description * Fixed the possible nil panicking issues with `to` package * Handled http connection source without httproute * Refactored test code ## Type of change <!-- Please select **one** of the following options that describes your change and delete the others. Clearly identifying the type of change you are making will help us review your PR faster, and is used in authoring release notes. If you are making a bug fix or functionality change to Radius and do not have an associated issue link please create one now. --> - This pull request fixes a bug in Radius and has an approved issue (issue link required). - This pull request adds or changes features of Radius and has an approved issue (issue link required). <!-- Please update the following to link the associated issue. This is required for some kinds of changes (see above). --> Fixes: radius-project#7004 radius-project#6937 --------- Signed-off-by: Young Bu Park <[email protected]>
# Description * Fixed the possible nil panicking issues with `to` package * Handled http connection source without httproute * Refactored test code ## Type of change <!-- Please select **one** of the following options that describes your change and delete the others. Clearly identifying the type of change you are making will help us review your PR faster, and is used in authoring release notes. If you are making a bug fix or functionality change to Radius and do not have an associated issue link please create one now. --> - This pull request fixes a bug in Radius and has an approved issue (issue link required). - This pull request adds or changes features of Radius and has an approved issue (issue link required). <!-- Please update the following to link the associated issue. This is required for some kinds of changes (see above). --> Fixes: radius-project#7004 radius-project#6937 --------- Signed-off-by: Young Bu Park <[email protected]> Signed-off-by: sk593 <[email protected]>
# Description * Fixed the possible nil panicking issues with `to` package * Handled http connection source without httproute * Refactored test code ## Type of change <!-- Please select **one** of the following options that describes your change and delete the others. Clearly identifying the type of change you are making will help us review your PR faster, and is used in authoring release notes. If you are making a bug fix or functionality change to Radius and do not have an associated issue link please create one now. --> - This pull request fixes a bug in Radius and has an approved issue (issue link required). - This pull request adds or changes features of Radius and has an approved issue (issue link required). <!-- Please update the following to link the associated issue. This is required for some kinds of changes (see above). --> Fixes: radius-project#7004 radius-project#6937 --------- Signed-off-by: Young Bu Park <[email protected]>
Description
topackageType of change
Fixes: #7004 #6937